feat(sessions): modernize conversation thread with collapsible tool groups#2673
Merged
Conversation
…roups
Unify every tool call (including MCP) onto one ToolRow primitive built on
base-ui Collapsible: a trigger (icon + text) plus a left-padded content box,
replacing the per-tool-view card markup.
A turn's tool-call work folds into a collapsible group chip with a verb-led
summary ("Ran 25 commands, read a file, edited a file") that shows the live
action while the turn runs. Collapse mode is persisted in settings
(all / collapse-when-finished / none, default all); per-chip overrides live in
session view state. Setup/clone progress and MCP-app tool calls stay always
visible so MCP iframes keep their mount.
Grouping is a single pass per token producing rows + keepMounted + id→row map,
with cached summaries for frozen turns and no per-scroll re-renders. Assistant
text inside a group is indented to the tool-title column. No feature flag.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
adamleithp
added
the
Stamphog
|
React Doctor found 7 issues in 3 files · 7 warnings. 7 warnings
Reviewed by React Doctor for commit |
![stamphog[bot]](https://avatars.githubusercontent.com/in/3000332?s=60&v=4){kind=small}
stamphog
Bot
removed
the
Stamphog
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Contributor
Prompt To Fix All With AIFix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/ui/src/features/sessions/components/session-update/ReadToolView.tsx:33-42
The image body carries its own `p-2` padding, but `ToolRow`'s content wrapper already adds `p-2` around every `content` node, so the image ends up with effectively `p-4` total padding instead of the previous `p-2`. The gray background (`bg-(--gray-2)`) is also applied only to the inner div rather than to the border box itself, leaving a white gap between the border and the gray area that didn't exist before.
```suggestion
const body = imageContent ? (
<SafeImagePreview
base64={imageContent.base64}
mimeType={imageContent.mimeType}
alt={filePath || "Read tool image preview"}
className="max-h-96 max-w-full object-contain"
/>
) : fileContent ? (
```
### Issue 2 of 2
packages/ui/src/features/sessions/components/new-thread/conversationThreadConfig.ts:67-77
**`messages` count is tracked but never surfaced**
`GroupCounts.messages` is incremented for `agent_message_chunk` and `console` session updates in `summarize()`, but `buildDoneLabel` references every other field (`execute`, `read`, `edit`, `delete`, `move`, `search`, `fetch`, `subagents`, `other`) and ignores `messages` entirely. The field is either dead (if mid-turn agent messages can't appear in `leading` after the trailing peel) or silently drops data that a future segment would need. Either remove the field from the interface or add a corresponding segment to `buildDoneLabel`.
Reviews (1): Last reviewed commit: "style(sessions): soften inline-code chip..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+33
to
+42
| const body = imageContent ? ( | ||
| <div className="bg-(--gray-2) p-2"> | ||
| <SafeImagePreview | ||
| base64={imageContent.base64} | ||
| mimeType={imageContent.mimeType} | ||
| alt={filePath || "Read tool image preview"} | ||
| className="max-h-96 max-w-full object-contain" | ||
| /> | ||
| </div> | ||
| ) : fileContent ? ( |
Contributor
There was a problem hiding this comment.
p-2 padding, but ToolRow's content wrapper already adds p-2 around every content node, so the image ends up with effectively p-4 total padding instead of the previous p-2. The gray background (bg-(--gray-2)) is also applied only to the inner div rather than to the border box itself, leaving a white gap between the border and the gray area that didn't exist before.
Suggested change
| const body = imageContent ? ( | |
| <div className="bg-(--gray-2) p-2"> | |
| <SafeImagePreview | |
| base64={imageContent.base64} | |
| mimeType={imageContent.mimeType} | |
| alt={filePath || "Read tool image preview"} | |
| className="max-h-96 max-w-full object-contain" | |
| /> | |
| </div> | |
| ) : fileContent ? ( | |
| const body = imageContent ? ( | |
| <SafeImagePreview | |
| base64={imageContent.base64} | |
| mimeType={imageContent.mimeType} | |
| alt={filePath || "Read tool image preview"} | |
| className="max-h-96 max-w-full object-contain" | |
| /> | |
| ) : fileContent ? ( |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/sessions/components/session-update/ReadToolView.tsx
Line: 33-42
Comment:
The image body carries its own `p-2` padding, but `ToolRow`'s content wrapper already adds `p-2` around every `content` node, so the image ends up with effectively `p-4` total padding instead of the previous `p-2`. The gray background (`bg-(--gray-2)`) is also applied only to the inner div rather than to the border box itself, leaving a white gap between the border and the gray area that didn't exist before.
```suggestion
const body = imageContent ? (
<SafeImagePreview
base64={imageContent.base64}
mimeType={imageContent.mimeType}
alt={filePath || "Read tool image preview"}
className="max-h-96 max-w-full object-contain"
/>
) : fileContent ? (
```
How can I resolve this? If you propose a fix, please make it concise.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
Comment on lines
+67
to
+77
| /** | ||
| * session_update kinds that never fold into a group — always rendered as | ||
| * their own row regardless of collapse mode (e.g. the cloud setup / clone | ||
| * progress card). | ||
| */ | ||
| alwaysVisibleUpdates: new Set<string>(["progress_group"]), | ||
| /** Max distinct tool icons shown in a collapsed chip's icon strip. */ | ||
| maxIconsInChip: 10, | ||
| } as const; | ||
|
|
||
| /** Per-action tallies for a collapsed group. */ |
Contributor
There was a problem hiding this comment.
messages count is tracked but never surfaced
GroupCounts.messages is incremented for agent_message_chunk and console session updates in summarize(), but buildDoneLabel references every other field (execute, read, edit, delete, move, search, fetch, subagents, other) and ignores messages entirely. The field is either dead (if mid-turn agent messages can't appear in leading after the trailing peel) or silently drops data that a future segment would need. Either remove the field from the interface or add a corresponding segment to buildDoneLabel.
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/ui/src/features/sessions/components/new-thread/conversationThreadConfig.ts
Line: 67-77
Comment:
**`messages` count is tracked but never surfaced**
`GroupCounts.messages` is incremented for `agent_message_chunk` and `console` session updates in `summarize()`, but `buildDoneLabel` references every other field (`execute`, `read`, `edit`, `delete`, `move`, `search`, `fetch`, `subagents`, `other`) and ignores `messages` entirely. The field is either dead (if mid-turn agent messages can't appear in `leading` after the trailing peel) or silently drops data that a future segment would need. Either remove the field from the interface or add a corresponding segment to `buildDoneLabel`.
How can I resolve this? If you propose a fix, please make it concise.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Rebuilds the conversation thread around a single, consistent tool-call row.
ToolRowprimitive (base-ui Collapsible): trigger = icon + text, content = left-padded box. Every tool view — execute, read, edit, search, fetch, think, MCP, generic, thinking — renders through it, replacing the old per-view card markup.Ran 25 commands, read a file, edited a file) that shows the live action while running. Persisted collapse mode in Settings: All collapsed (default) / Collapse when finished turn / No collapsing. Per-chip expand/collapse overrides held in session view state.progress_group) and MCP-app tool calls stay outside groups so MCP iframes keep their mount (keepMounted).Perf
keepMounted+ id→row map (was several walks), with cached summaries for frozen turns.Notes
mergeConversationItemsdedup gap when an attachment is inlined as text vs. a chip). Not touched here.🤖 Generated with Claude Code